Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade EUI to v95.9.0 #190752

Merged
merged 25 commits into from
Aug 31, 2024
Merged

Upgrade EUI to v95.9.0 #190752

merged 25 commits into from
Aug 31, 2024

Conversation

cee-chen
Copy link
Member

@cee-chen cee-chen commented Aug 20, 2024

v95.7.0v95.9.0

Caution

This PR contains the final set of Emotion conversions for all EuiForm components.
If your plugin contains any custom CSS/styling to EuiFormRow, EuiFormControlLayout, EuiCheckbox, EuiRadio, or EuiSwitch, ⚠️ make sure to QA your UI to ensure no visual regressions have occurred! ⚠️


v95.9.0

  • Updated EuiSearchBar's optional box.schema prop with a new recognizedFields configuration. This allows specifying the phrases that will be parsed as field clauses (#7960)
  • Updated EuiIcon with a new tokenSemanticText glyph (#7971)
  • Added support for TypeScript 5 (#7980)

Bug fixes

  • Fixed EuiSelectableTemplateSitewide styles when used within a dark-themed EuiHeader (#7977)

v95.8.0

  • Updated EuiHeaderLinks's mobile menu to set a slight popover padding by default (#7961)
    • This can be overridden via popoverProps.panelPaddingSize if needed.
  • Updated EuiHeaderLink to default to a size of s (down from m) (#7961)

Accessibility

  • Updated the aria-label attribute for the EuiFieldSearch clear button (#7970)

Bug fixes

  • Fixed a visual bug with <EuiDualRange showInput="inputWithPopover" /> form controls (#7957)

Deprecations

  • Deprecated EuiFormRow's columnCompressedSwitch display prop. Use columnCompressed instead, which will automatically account for child EuiSwitches (#7968)
  • Deprecated EuiFormRow's rowCompressed display prop. Use row instead for vertical forms, or centerCompressed for inline forms (#7968)
  • (Styling) Updated EuiFormRow's hasEmptySpaceLabel prop to no longer attempt to automatically align its content to a vertical center. Use the display="center" prop for that instead (#7968)

CSS-in-JS conversions

  • Converted EuiFormControlLayout to Emotion (#7954)
    • Removed .euiFormControlLayout--*icons classNames and --eui-form-control-layout-icons-padding CSS var. Use --euiFormControlRightIconsCount or --euiFormControlLeftIconsCount instead
  • Converted EuiFormLayoutDelimited to Emotion (#7957)
  • Fixed cloneElementWithCss throwing an error when used multiple times without a key prop (#7957)
  • Updated cloneElementWithCss utility to support a third argument that allows prepending vs. appending the cloned Emotion css className (#7957)
  • Removed @euiFormControlLayoutClearIcon Sass mixin (#7959)
  • Converted EuiDescribedFormGroup to Emotion (#7964)
  • Converted EuiForm, EuiFormHelpText, and EuiFormErrorText to Emotion (#7966)
  • Converted EuiFormLabel and EuiFormLegend to Emotion; Removed @euiFormLabel mixin (#7967)
  • Converted EuiFormRow to Emotion (#7968)
  • Converted EuiCheckbox to Emotion (#7969)
  • Converted EuiRadio to Emotion (#7969)
  • Converted EuiSwitch to Emotion (#7969)
  • Removed the following Sass variables: (#7969)
    • $euiFormCustomControlDisabledIconColor
    • $euiFormCustomControlBorderColor
    • $euiRadioSize
    • $euiCheckBoxSize
    • $euiCheckboxBorderRadius
    • $euiSwitchHeight (and compressed/mini variants)
    • $euiSwitchWidth (and compressed/mini variants)
    • $euiSwitchThumbSize (and compressed/mini variants)
    • $euiSwitchIconHeight
    • $euiSwitchOffColor
  • Removed the following Sass mixins: (#7969)
    • euiIconBackground
    • euiCustomControl
    • euiCustomControlSelected
    • euiCustomControlDisabled
    • euiCustomControlFocused

- caused by additional Emotion wrapper that matches the same selector
…ration

- default props are no longer read/rendered by enzyme's snapshotter
…to `useGeneratedHtmlId`

- requires updating mocks
- checkbox and radio inputs should now be directly clickable, and have a different DOM wrapper
- use EuiTitle size and font weight instead
- `.spcShareToSpaceIncludeRelated` isn't actually used anywhere in Kibana
- EuiCheckbox DOM has changed since, color: inherit should work for both disabled and non-disabled checkboxes
…ons with components instead

- in the cast of the `prepend` prop, `<EuiFormLabel>` is already automatically used if the prepend type is a string
- EuiFormControlLayout should automatically handle styling, and prepend/append nodes shouldn't be used outside of form layouts
…Group`s

- EuiDescribedFormGroups already automatically accounts for form rows without labels, so the `hasEmptyLabelSpace` prop is now unnecessary and adds extra unwanted spacing
@cee-chen cee-chen marked this pull request as ready for review August 24, 2024 00:37
@cee-chen
Copy link
Member Author

@Heenawter This should be fixed now from ebce0ce with some extra UI/UX pizazz added to the warning underline for the RangeSliderControl component, as we chatted about over Slack!



Let me know if you have any questions about the changes or if you spot any UI bugs that I missed.

Copy link
Contributor

@criamico criamico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally Fleet and Integrations and looks good! LGTM

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For ML, spotted one regression.

Copy link
Contributor

@mbondyra mbondyra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Visualizations team changes LGTM! Thank you!

- doesn't apply and isn't necessary, removing it gets the button group back to the previous prod rendering
Copy link
Contributor

@Heenawter Heenawter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that the border on the controls is noticeably lighter in this PR than on main:

  • Before:
    image

  • After:
    image

Not a big deal, but is this an intentional change?

Either way, approving to unblock, since this is pretty minor - thanks so much for your effort in cleaning up our styling, it is very, very much appreciated 🙇 The new range slider styling is so clean 🤌

@cee-chen
Copy link
Member Author

I noticed that the border on the controls is noticeably lighter in this PR than on main [...] is this an intentional change?

This is an intentional change! Borders for inputs with prepend/appends (aka grouped inputs) were previously darker than inputs without them. This is now fixed so there is no change in border lightness/darkness between grouped inputs and non-grouped inputs.

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for ML. Thanks for fixing the Smart grouping field!

Copy link
Contributor

@Samiul-TheSoccerFan Samiul-TheSoccerFan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, code review only.

Only one file has been updated x-pack/plugins/enterprise_search/public/applications/workplace_search/views/content_sources/components/synchronization/blocked_window_item.test.tsx

Copy link
Member

@robbavey robbavey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cee-chen cee-chen enabled auto-merge (squash) August 30, 2024 16:01
@kibana-ci
Copy link
Collaborator

kibana-ci commented Aug 30, 2024

💛 Build succeeded, but was flaky

  • Buildkite Build
  • Commit: 035a5c1
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-190752-035a5c17ef64

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
controls 302 437 +135
spaces 277 272 -5
total +130

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
aiops 547.5KB 547.4KB -97.0B
cases 480.2KB 480.1KB -42.0B
controls 380.5KB 561.5KB +181.0KB
infra 1.5MB 1.5MB +118.0B
kubernetesSecurity 212.4KB 212.5KB +118.0B
lens 1.5MB 1.5MB -59.0B
maps 3.0MB 3.0MB -120.0B
ml 4.6MB 4.6MB -60.0B
presentationUtil 85.9KB 85.9KB -30.0B
remoteClusters 78.9KB 78.8KB -66.0B
snapshotRestore 262.1KB 261.5KB -594.0B
spaces 185.1KB 184.4KB -796.0B
stackConnectors 580.3KB 580.2KB -48.0B
unifiedSearch 219.0KB 218.7KB -346.0B
visTypeVega 1.8MB 1.8MB +118.0B
total +179.2KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
controls 57.7KB 57.8KB +88.0B
core 418.5KB 418.6KB +141.0B
kbnUiSharedDeps-css 178.4KB 147.8KB -30.6KB
kbnUiSharedDeps-npmDll 6.2MB 6.2MB +26.9KB
snapshotRestore 27.6KB 27.0KB -606.0B
total -4.1KB
Unknown metric groups

async chunk count

id before after diff
controls 16 17 +1

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@Ikuni17 Ikuni17 disabled auto-merge August 31, 2024 00:47
@Ikuni17 Ikuni17 merged commit a88cfc8 into elastic:main Aug 31, 2024
50 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Aug 31, 2024
@cee-chen cee-chen deleted the eui/v95.8.0 branch August 31, 2024 05:48
Ikuni17 pushed a commit that referenced this pull request Sep 24, 2024
## Summary

This is a follow up to EUI's Emotion conversion of
**EuiFormControlLayout/Delimited** (see
#190752,
elastic/eui#7954, and
elastic/eui#7957).

> [!note]
> Please manually QA your team's affected form control(s) to confirm
they still look and behave as expected and are non-broken. The EUI team
is not familiar enough with each plugin's setups to pull down and QA
this PR ourselves.

While QA testing the upgrade, I noticed a few incorrect usages of
**EuiFormControlLayout** but wanted to wait until after the upgrade to
push out fixes (to prevent delaying the PR further). In general, here is
EUI's [recommended usage of the
component](https://eui.elastic.co/#/forms/form-controls#form-control-layout):

- Where possible, **simply don't use it**. Almost all form controls are
**already** automatically wrapped in any EuiFormControlLayout by
default, and should accept a large majority of the props that the layout
accepts.
- If you **must** use it, set the `controlOnly` prop on the child
input/control to avoid buggy styling (e.g. duplicate borders).
- If you can't do either of the above for any reason (e.g. missing prop
support), reach out to the EUI team to ask for your UX as a feature
request!

### Checklist

Delete any items that are not applicable to this PR.

- [x] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [x] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [x] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 24, 2024
## Summary

This is a follow up to EUI's Emotion conversion of
**EuiFormControlLayout/Delimited** (see
elastic#190752,
elastic/eui#7954, and
elastic/eui#7957).

> [!note]
> Please manually QA your team's affected form control(s) to confirm
they still look and behave as expected and are non-broken. The EUI team
is not familiar enough with each plugin's setups to pull down and QA
this PR ourselves.

While QA testing the upgrade, I noticed a few incorrect usages of
**EuiFormControlLayout** but wanted to wait until after the upgrade to
push out fixes (to prevent delaying the PR further). In general, here is
EUI's [recommended usage of the
component](https://eui.elastic.co/#/forms/form-controls#form-control-layout):

- Where possible, **simply don't use it**. Almost all form controls are
**already** automatically wrapped in any EuiFormControlLayout by
default, and should accept a large majority of the props that the layout
accepts.
- If you **must** use it, set the `controlOnly` prop on the child
input/control to avoid buggy styling (e.g. duplicate borders).
- If you can't do either of the above for any reason (e.g. missing prop
support), reach out to the EUI team to ask for your UX as a feature
request!

### Checklist

Delete any items that are not applicable to this PR.

- [x] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [x] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [x] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

(cherry picked from commit fd7b86e)
kibanamachine added a commit that referenced this pull request Sep 24, 2024
# Backport

This will backport the following commits from `main` to `8.x`:
- [Fix various EuiFormControlLayout usages
(#192779)](#192779)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Cee
Chen","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-09-24T20:55:59Z","message":"Fix
various EuiFormControlLayout usages (#192779)\n\n## Summary\r\n\r\nThis
is a follow up to EUI's Emotion conversion
of\r\n**EuiFormControlLayout/Delimited**
(see\r\nhttps://github.com//pull/190752,\r\nhttps://github.com/elastic/eui/pull/7954,
and\r\nhttps://github.com/elastic/eui/pull/7957).\r\n\r\n> [!note]\r\n>
Please manually QA your team's affected form control(s) to
confirm\r\nthey still look and behave as expected and are non-broken.
The EUI team\r\nis not familiar enough with each plugin's setups to pull
down and QA\r\nthis PR ourselves.\r\n\r\nWhile QA testing the upgrade, I
noticed a few incorrect usages of\r\n**EuiFormControlLayout** but wanted
to wait until after the upgrade to\r\npush out fixes (to prevent
delaying the PR further). In general, here is\r\nEUI's [recommended
usage of
the\r\ncomponent](https://eui.elastic.co/#/forms/form-controls#form-control-layout):\r\n\r\n-
Where possible, **simply don't use it**. Almost all form controls
are\r\n**already** automatically wrapped in any EuiFormControlLayout
by\r\ndefault, and should accept a large majority of the props that the
layout\r\naccepts.\r\n- If you **must** use it, set the `controlOnly`
prop on the child\r\ninput/control to avoid buggy styling (e.g.
duplicate borders).\r\n- If you can't do either of the above for any
reason (e.g. missing prop\r\nsupport), reach out to the EUI team to ask
for your UX as a feature\r\nrequest!\r\n\r\n### Checklist\r\n\r\nDelete
any items that are not applicable to this PR.\r\n\r\n- [x] Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard accessibility](https://webaim.org/techniques/keyboard/))\r\n-
[x] Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[x] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[x] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)","sha":"fd7b86e209e7133f3d9b7bd4e9fd6542f8a3aaad","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","EUI","v9.0.0","backport:prev-minor","ci:project-deploy-observability","Team:obs-ux-infra_services","Team:obs-ux-management","apm:review","v8.16.0"],"title":"Fix
various EuiFormControlLayout
usages","number":192779,"url":"https://github.com/elastic/kibana/pull/192779","mergeCommit":{"message":"Fix
various EuiFormControlLayout usages (#192779)\n\n## Summary\r\n\r\nThis
is a follow up to EUI's Emotion conversion
of\r\n**EuiFormControlLayout/Delimited**
(see\r\nhttps://github.com//pull/190752,\r\nhttps://github.com/elastic/eui/pull/7954,
and\r\nhttps://github.com/elastic/eui/pull/7957).\r\n\r\n> [!note]\r\n>
Please manually QA your team's affected form control(s) to
confirm\r\nthey still look and behave as expected and are non-broken.
The EUI team\r\nis not familiar enough with each plugin's setups to pull
down and QA\r\nthis PR ourselves.\r\n\r\nWhile QA testing the upgrade, I
noticed a few incorrect usages of\r\n**EuiFormControlLayout** but wanted
to wait until after the upgrade to\r\npush out fixes (to prevent
delaying the PR further). In general, here is\r\nEUI's [recommended
usage of
the\r\ncomponent](https://eui.elastic.co/#/forms/form-controls#form-control-layout):\r\n\r\n-
Where possible, **simply don't use it**. Almost all form controls
are\r\n**already** automatically wrapped in any EuiFormControlLayout
by\r\ndefault, and should accept a large majority of the props that the
layout\r\naccepts.\r\n- If you **must** use it, set the `controlOnly`
prop on the child\r\ninput/control to avoid buggy styling (e.g.
duplicate borders).\r\n- If you can't do either of the above for any
reason (e.g. missing prop\r\nsupport), reach out to the EUI team to ask
for your UX as a feature\r\nrequest!\r\n\r\n### Checklist\r\n\r\nDelete
any items that are not applicable to this PR.\r\n\r\n- [x] Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard accessibility](https://webaim.org/techniques/keyboard/))\r\n-
[x] Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[x] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[x] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)","sha":"fd7b86e209e7133f3d9b7bd4e9fd6542f8a3aaad"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/192779","number":192779,"mergeCommit":{"message":"Fix
various EuiFormControlLayout usages (#192779)\n\n## Summary\r\n\r\nThis
is a follow up to EUI's Emotion conversion
of\r\n**EuiFormControlLayout/Delimited**
(see\r\nhttps://github.com//pull/190752,\r\nhttps://github.com/elastic/eui/pull/7954,
and\r\nhttps://github.com/elastic/eui/pull/7957).\r\n\r\n> [!note]\r\n>
Please manually QA your team's affected form control(s) to
confirm\r\nthey still look and behave as expected and are non-broken.
The EUI team\r\nis not familiar enough with each plugin's setups to pull
down and QA\r\nthis PR ourselves.\r\n\r\nWhile QA testing the upgrade, I
noticed a few incorrect usages of\r\n**EuiFormControlLayout** but wanted
to wait until after the upgrade to\r\npush out fixes (to prevent
delaying the PR further). In general, here is\r\nEUI's [recommended
usage of
the\r\ncomponent](https://eui.elastic.co/#/forms/form-controls#form-control-layout):\r\n\r\n-
Where possible, **simply don't use it**. Almost all form controls
are\r\n**already** automatically wrapped in any EuiFormControlLayout
by\r\ndefault, and should accept a large majority of the props that the
layout\r\naccepts.\r\n- If you **must** use it, set the `controlOnly`
prop on the child\r\ninput/control to avoid buggy styling (e.g.
duplicate borders).\r\n- If you can't do either of the above for any
reason (e.g. missing prop\r\nsupport), reach out to the EUI team to ask
for your UX as a feature\r\nrequest!\r\n\r\n### Checklist\r\n\r\nDelete
any items that are not applicable to this PR.\r\n\r\n- [x] Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard accessibility](https://webaim.org/techniques/keyboard/))\r\n-
[x] Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[x] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[x] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)","sha":"fd7b86e209e7133f3d9b7bd4e9fd6542f8a3aaad"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Cee Chen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting ci:project-deploy-observability Create an Observability project EUI release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v8.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.